pkg/aiusechat/usechat.go (1)

313-319: Persist initial ToolUseData to chat store as well

Pre-emitting SSE is great. Consider also persisting the initial ToolUseData so a reconnect/history load reflects it without relying on prior SSE.

Apply this diff to also update the chat:

 for _, toolCall := range stopReason.ToolCalls {
     if toolCall.ToolUseData != nil {
         log.Printf("AI data-tooluse %s\n", toolCall.ID)
         _ = sseHandler.AiMsgData("data-tooluse", toolCall.ID, *toolCall.ToolUseData)
+        updateToolUseDataInChat(chatOpts, toolCall.ID, toolCall.ToolUseData)
     }
 }

frontend/app/aipanel/aimessage.tsx (2)

121-127: Group title can be wrong when mixing file reads and directory listings

A single batch may contain both read_text_file and read_dir; using the first item to pick title can mislabel the group.

Split batches by toolname (reads vs listings) or compute a combined label, e.g., “Reading files and listing directories”.

---

128-149: Prefer stable keys (toolcallid) over array indices

Index keys cause unnecessary re-renders and can break UI state when arrays change.

Apply this diff:

-                    {parts.map((part, idx) => {
+                    {parts.map((part) => {
@@
-                            <div key={idx} className="text-sm pl-2">
+                            <div key={part.data.toolcallid} className="text-sm pl-2">

-            {otherTools.map((tool, idx) => (
-                <div key={idx} className="mt-2">
+            {otherTools.map((tool) => (
+                <div key={tool.data.toolcallid} className="mt-2">

Also applies to: 290-293

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

frontend/app/aipanel/aimessage.tsx (1)

121-123: Batch title can be inaccurate for mixed file ops

If the batch contains both read_text_file and read_dir, the title uses only the first item.

Replace with:

-    const groupTitle = firstTool.toolname === "read_text_file" ? "Reading Files" : "Listing Directories";
+    const hasReadFile = parts.some((p) => p.data.toolname === "read_text_file");
+    const hasReadDir = parts.some((p) => p.data.toolname === "read_dir");
+    const groupTitle = hasReadFile && hasReadDir ? "File Access" : hasReadFile ? "Reading Files" : "Listing Directories";

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

frontend/app/aipanel/aimessage.tsx (2)

135-158: Fix Rule of Hooks violation and use “any needs approval” for keepalive

• useEffect is declared after an early return; violates hook order.
• Keepalive is gated by group-level approval based on the first tool; others may starve. Use anyNeedApproval.

Apply:

 const AIToolUseBatch = memo(({ parts, isStreaming }: AIToolUseBatchProps) => {
-    const [userApprovalOverride, setUserApprovalOverride] = useState<string | null>(null);
-
-    if (parts.length === 0) return null;
-
-    const firstTool = parts[0].data;
-    const baseApproval = userApprovalOverride || firstTool.approval;
-    const effectiveApproval = !isStreaming && baseApproval === "needs-approval" ? "timeout" : baseApproval;
-    const allNeedApproval = parts.every((p) => (userApprovalOverride || p.data.approval) === "needs-approval");
-
-    useEffect(() => {
-        if (!isStreaming || effectiveApproval !== "needs-approval") return;
+    const [userApprovalOverride, setUserApprovalOverride] = useState<string | null>(null);
+    const hasParts = parts.length > 0;
+    const anyNeedApproval = parts.some((p) => (userApprovalOverride || p.data.approval) === "needs-approval");
+    const allNeedApproval = parts.every((p) => (userApprovalOverride || p.data.approval) === "needs-approval");
+
+    useEffect(() => {
+        if (!isStreaming || !anyNeedApproval) return;
 
         const interval = setInterval(() => {
             parts.forEach((part) => {
                 RpcApi.WaveAIToolApproveCommand(TabRpcClient, {
                     toolcallid: part.data.toolcallid,
                     keepalive: true,
                 });
             });
         }, 4000);
 
         return () => clearInterval(interval);
-    }, [isStreaming, effectiveApproval, parts]);
+    }, [isStreaming, anyNeedApproval, parts]);
+
+    if (!hasParts) return null;

Based on static analysis hints

---

184-191: Per-part approval for status/error and approval UI gating

• Items currently receive a group-level effectiveApproval; errors/timeouts can be misreported.
• Show Approve All only when all parts still need approval.

Apply:

-                <div className="mt-1 space-y-0.5">
-                    {parts.map((part, idx) => (
-                        <AIToolUseBatchItem key={idx} part={part} effectiveApproval={effectiveApproval} />
-                    ))}
-                </div>
-                {allNeedApproval && effectiveApproval === "needs-approval" && (
-                    <AIToolApprovalButtons count={parts.length} onApprove={handleApprove} onDeny={handleDeny} />
-                )}
+                <div className="mt-1 space-y-0.5">
+                    {parts.map((part, idx) => {
+                        const partBaseApproval = userApprovalOverride || part.data.approval;
+                        const partEffectiveApproval =
+                            !isStreaming && partBaseApproval === "needs-approval"
+                                ? "timeout"
+                                : partBaseApproval;
+                        return (
+                            <AIToolUseBatchItem
+                                key={part.data.toolcallid ?? idx}
+                                part={part}
+                                effectiveApproval={partEffectiveApproval}
+                            />
+                        );
+                    })}
+                </div>
+                {allNeedApproval && (
+                    <AIToolApprovalButtons count={parts.length} onApprove={handleApprove} onDeny={handleDeny} />
+                )}

frontend/app/aipanel/aimessage.tsx (1)

249-251: Optional: show explicit message when user denies

Currently only timeouts show “Not approved.” Consider surfacing “Denied by user” for clarity.

-                {(toolData.errormessage || effectiveApproval === "timeout") && (
-                    <div className="text-sm text-red-300 mt-1">{toolData.errormessage || "Not approved"}</div>
-                )}
+                {(toolData.errormessage || effectiveApproval === "timeout" || (userApprovalOverride || toolData.approval) === "user-denied") && (
+                    <div className="text-sm text-red-300 mt-1">
+                        {toolData.errormessage ||
+                            ((userApprovalOverride || toolData.approval) === "user-denied" ? "Denied by user" : "Not approved")}
+                    </div>
+                )}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

electron.vite.config.ts (1)

133-147: Package name patterns still miss scoped packages and risk false positives.

This issue was flagged in two previous review comments. The current substring patterns:

• Check for monaco instead of the actual package monaco-editor
• Check for @monaco instead of the actual scope @monaco-editor
• Check for @mermaid instead of the actual scope @mermaid-js
• Check for @shiki instead of the actual scope @shikijs

While substring matching may catch some cases, it's imprecise and could incorrectly chunk unrelated packages (e.g., a hypothetical monaco-fork or @monaco-custom).

Apply this diff to use precise package names and scopes:

                 output: {
                     manualChunks(id) {
                         const p = id.replace(/\\/g, "/");
-                        if (p.includes("node_modules/monaco") || p.includes("node_modules/@monaco")) return "monaco";
-                        if (p.includes("node_modules/mermaid") || p.includes("node_modules/@mermaid"))
+                        if (p.includes("/node_modules/monaco-editor") || p.includes("/node_modules/@monaco-editor/"))
+                            return "monaco";
+                        if (p.includes("/node_modules/mermaid") || p.includes("/node_modules/@mermaid-js/"))
                             return "mermaid";
-                        if (p.includes("node_modules/katex") || p.includes("node_modules/@katex")) return "katex";
-                        if (p.includes("node_modules/shiki") || p.includes("node_modules/@shiki")) {
+                        if (p.includes("/node_modules/katex") || p.includes("/node_modules/@katex/")) return "katex";
+                        if (p.includes("/node_modules/shiki") || p.includes("/node_modules/@shikijs/")) {
                             return "shiki";
                         }
-                        if (p.includes("node_modules/cytoscape") || p.includes("node_modules/@cytoscape"))
+                        if (p.includes("/node_modules/cytoscape") || p.includes("/node_modules/@cytoscape/"))
                             return "cytoscape";
                         return undefined;
                     },
                 },

frontend/app/aipanel/aimessage.tsx (5)

83-95: Explicit button type to avoid accidental form submit

Add type="button" to both buttons (defensive best practice).

-            <button
+            <button
+                type="button"
                 onClick={onApprove}
@@
-            <button
+            <button
+                type="button"
                 onClick={onDeny}

---

101-128: Per-item timeout/error labeling (future-proof)

Batch currently passes group-wide effectiveApproval to each item. Given batches are split by approval, this is fine. To future-proof for mixed states, derive per-item effectiveApproval in the map and pass it down.

Based on learnings

-interface AIToolUseBatchItemProps {
-    part: WaveUIMessagePart & { type: "data-tooluse" };
-    effectiveApproval: string;
-}
+interface AIToolUseBatchItemProps {
+    part: WaveUIMessagePart & { type: "data-tooluse" };
+    effectiveApproval: string; // now computed per-item
+}

And where items are rendered (see lines 182-184):

-    {parts.map((part, idx) => (
-        <AIToolUseBatchItem key={idx} part={part} effectiveApproval={effectiveApproval} />
-    ))}
+    {parts.map((part, idx) => {
+        const partBase = (userApprovalOverride || part.data.approval) as string;
+        const partEff =
+            !isStreaming && partBase === "needs-approval" ? "timeout" : partBase;
+        return (
+            <AIToolUseBatchItem key={idx} part={part} effectiveApproval={partEff} />
+        );
+    })}

---

134-155: Hooks order: looks good; optional defensive guard

No early return before hooks anymore. Since AIToolUseBatch assumes non-empty parts (and AIToolUseGroup enforces it), this is safe. Optionally, add a post-effect guard to avoid parts[0] access if reused elsewhere.

Based on learnings

 const AIToolUseBatch = memo(({ parts, isStreaming }: AIToolUseBatchProps) => {
     const [userApprovalOverride, setUserApprovalOverride] = useState<string | null>(null);
+    const hasParts = parts.length > 0;
 
-    const firstTool = parts[0].data;
+    // effect first; guards on hasParts
     useEffect(() => {
-        if (!isStreaming || effectiveApproval !== "needs-approval") return;
+        if (!isStreaming || !hasParts) return;
+        // rest unchanged…
     }, [isStreaming, /* include */ hasParts, /* …existing deps */]);
 
+    if (!hasParts) return null;
+    const firstTool = parts[0].data;

---

354-379: Consider memoizing grouping

groupMessageParts runs each render. For long message streams, memoize on displayParts reference.

-import { memo, useEffect, useState } from "react";
+import { memo, useEffect, useMemo, useState } from "react";
@@
-const groupedParts = groupMessageParts(displayParts);
+const groupedParts = useMemo(() => groupMessageParts(displayParts), [displayParts]);

---

434-446: Use stable keys to reduce re-mounts during streaming

Index keys can cause unnecessary remounts when parts are appended/mutated. Prefer stable keys.

-{groupedParts.map((group, index: number) =>
-    group.type === "toolgroup" ? (
-        <AIToolUseGroup key={index} parts={group.parts} isStreaming={isStreaming} />
-    ) : (
-        <div key={index} className="mt-2">
-            <AIMessagePart part={group.part} role={message.role} isStreaming={isStreaming} />
-        </div>
-    )
-)}
+{groupedParts.map((group, index: number) => {
+    const key =
+        group.type === "toolgroup"
+            ? group.parts.map((p) => (p as any).data?.toolcallid || index).join(",")
+            : ((group.part as any).data?.toolcallid ?? `single-${index}`);
+    return group.type === "toolgroup" ? (
+        <AIToolUseGroup key={key} parts={group.parts} isStreaming={isStreaming} />
+    ) : (
+        <div key={key} className="mt-2">
+            <AIMessagePart part={group.part} role={message.role} isStreaming={isStreaming} />
+        </div>
+    );
+})}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: sawka
PR: wavetermdev/waveterm#2430
File: frontend/app/aipanel/aimessage.tsx:137-144
Timestamp: 2025-10-14T06:30:54.751Z
Learning: In `frontend/app/aipanel/aimessage.tsx`, the `AIToolUseGroup` component splits file operation tool calls into separate batches (`fileOpsNeedApproval` and `fileOpsNoApproval`) based on their approval state before passing them to `AIToolUseBatch`. This ensures each batch has homogeneous approval states, making group-level approval handling valid.

Learnt from: sawka
PR: wavetermdev/waveterm#2430
File: frontend/app/aipanel/aimessage.tsx:137-144
Timestamp: 2025-10-14T06:30:54.751Z
Learning: In `frontend/app/aipanel/aimessage.tsx`, the `AIToolUseGroup` component splits file operation tool calls into separate batches (`fileOpsNeedApproval` and `fileOpsNoApproval`) based on their approval state before passing them to `AIToolUseBatch`. This ensures each batch has homogeneous approval states, making group-level approval handling valid.